Parametric google-test for mapping validation by JoergAtGithub · Pull Request #13281 · mixxxdj/mixxx

JoergAtGithub · 2024-05-25T13:38:07Z

For debugging #11625 (comment) I modified the googletest setup for mapping validation to use a parametrized list of mappings.
While the results look nice in my IDE, where I can easily debug a single mapping validation test now:

I was disappointed to see that CTest combines all the single GTest instances:

        Start 121: */LegacyControllerMappingValidationTest.ValidateMappingXML/*
121/840 Test #121: */LegacyControllerMappingValidationTest.ValidateMappingXML/* ...............................   Passed    0.37 sec
        Start 122: */LegacyControllerMappingValidationTest.LoadMapping/*
122/840 Test #122: */LegacyControllerMappingValidationTest.LoadMapping/* ......................................***Failed    5.44 sec

I could solve that by using gtest_discover_tests in the CMakelist.txt, now the CTest output looks like this:

          Start 1039: MidiMappingsValid/LegacyControllerMappingValidationTest.LoadMapping/Pioneer_CDJ_350_CH2#GetParam()=<MappingInfo>
1039/1107 Test #1039: MidiMappingsValid/LegacyControllerMappingValidationTest.LoadMapping/Pioneer_CDJ_350_CH2#GetParam()=<MappingInfo> .................................   Passed    0.45 sec
          Start 1040: MidiMappingsValid/LegacyControllerMappingValidationTest.LoadMapping/Pioneer_CDJ_850#GetParam()=<MappingInfo>
1040/1107 Test #1040: MidiMappingsValid/LegacyControllerMappingValidationTest.LoadMapping/Pioneer_CDJ_850#GetParam()=<MappingInfo> .....................................   Passed    0.45 sec
          Start 1041: MidiMappingsValid/LegacyControllerMappingValidationTest.LoadMapping/Pioneer_DDJ_200#GetParam()=<MappingInfo>
1041/1107 Test #1041: MidiMappingsValid/LegacyControllerMappingValidationTest.LoadMapping/Pioneer_DDJ_200#GetParam()=<MappingInfo> .....................................   Passed    0.53 sec
          Start 1042: MidiMappingsValid/LegacyControllerMappingValidationTest.LoadMapping/Pioneer_DDJ_400#GetParam()=<MappingInfo>
1042/1107 Test #1042: MidiMappingsValid/LegacyControllerMappingValidationTest.LoadMapping/Pioneer_DDJ_400#GetParam()=<MappingInfo> .....................................   Passed    0.45 sec
          Start 1043: MidiMappingsValid/LegacyControllerMappingValidationTest.LoadMapping/Pioneer_DDJ_FLX4#GetParam()=<MappingInfo>
1043/1107 Test #1043: MidiMappingsValid/LegacyControllerMappingValidationTest.LoadMapping/Pioneer_DDJ_FLX4#GetParam()=<MappingInfo> ....................................   Passed    0.47 sec

…all mappings

Fixed clazy error

…ized gtests

…atrix

…appings

github-actions · 2024-09-24T00:09:18Z

This PR is marked as stale because it has been open 90 days with no activity.

…appings

…s slow

github-actions · 2025-01-23T00:09:22Z

This PR is marked as stale because it has been open 90 days with no activity.

…all mappings

Fixed clazy error

acolombier · 2025-11-22T19:17:34Z

I think we should get this improvement in! I have taken some time to rebase this branch against 2.6 and add some minor improvement, related to @Swiftb0y suggestions

9f99d0158f

Let me know if you want me to submit it in a PR directly.

JoergAtGithub · 2025-11-22T19:46:51Z

The problem is that this PR slows down the test significiantly, because creating the list of mappings takes a lot of time. A hardcoded list shouldn't have this issue.
Otherwise thi PR works.

Swiftb0y · 2025-11-22T20:21:20Z

what magnitude of slowdown are we talking about here? seconds?

JoergAtGithub · 2025-11-22T20:26:34Z

The whole testsuite needs about 2x time as before, because the list of mappings is calculate before every test (not only the test that depend on this parameter)

Swiftb0y · 2025-11-22T20:49:45Z

oh yeah, then that is a significant problem.

acolombier · 2025-11-23T01:34:44Z

I have pushed a version with the mapping list computed directly in CMake. Only did MIDI as example, but keen to hear if we would want to go ahead with it. There is undoubtedly an overhead with running multiple tests, perhaps we could make it optional, so only the coverage job would run the test in parametrised mode and the slowdown wouldn't impact the build job?

JoergAtGithub · 2025-11-23T07:25:53Z

GTest must always work standalone without CTest, as GTest is what is used in IDEs. One of the challenges to achieve this ist to find the ./res/controller directory in all environments.

acolombier · 2025-11-23T11:34:11Z

Yes, this is what my solution does. It only finds the mapping at compile time to build the parameter list. Once built, mixxx-test can be used independently to CTest, and should work fine in the IDE as it will find the res/controller directory in the same way it predictably find src

…ppings

Use CMake to collect mapping files at configure time and pass them compile definition.

…rollers/midi-components-0.0.js:140:\nTypeError: Cannot call method 'trigger' of undefined\n\nBacktrace: @file:///home/runner/work/mixxx/mixxx/res/controllers/midi-components-0.0.js:140\ntrigger@file:///home/runner/work/mixxx/mixxx/res/controllers/midi-components-0.0.js:139\nComponent@file:///home/runner/work/mixxx/mixxx/res/controllers/midi-components-0.0.js:53\nButton@file:///home/runner/work/mixxx/mixxx/res/controllers/midi-components-0.0.js:164\n%entry@file:///home/runner/work/mixxx/mixxx/res/controllers/Hercules-P32-scripts.js:109

JoergAtGithub · 2026-02-08T23:30:00Z

Finally ready, thanks to the suggestion by @acolombier above!

acolombier

Looking good, thanks for pushing this to the end! I haven't tested it yet

acolombier · 2026-02-08T23:39:17Z

            if (this.connections[0] !== undefined) {
                this.connections.forEach(function(conn) {
-                    conn.disconnect();
+                    if (conn !== undefined) {


Is this an expected usecase? It does makes sense in the case of testing due to missing CO, but I assume that connections should only contain valid connections, else it would indicate a problem.
Should we add a console.warn to flag potentially failed connection?

Detailed warnings are already implemented in engine.makeConnection:

mixxx/src/controllers/scripting/legacy/controllerscriptinterfacelegacy.cpp

Lines 276 to 290 in c10f342

if (coScript == nullptr) {

// The test setups do not run all of Mixxx, so ControlObjects not

// existing during tests is okay.

if (!m_pScriptEngineLegacy->isTesting()) {

m_pScriptEngineLegacy->logOrThrowError(

QStringLiteral("script tried to connect to ControlObject "

"(%1, %2) which is non-existent.")

.arg(group, name));

}

return QJSValue();

}

if (!callback.isCallable()) {

m_pScriptEngineLegacy->logOrThrowError(QStringLiteral(

"Tried to connect (%1, %2) to an invalid callback. Make sure "

But after the warning engine.makeConnection returns undefined and the script continues.

acolombier · 2026-02-08T23:42:36Z

+    PUBLIC CONTROLLER_HID_MAPPINGS="${CONTROLLER_HID_MAPPINGS}"
+    PUBLIC CONTROLLER_MIDI_MAPPINGS="${CONTROLLER_MIDI_MAPPINGS}"
+  )
+


DO we need to make the above conditional to HID, and BULK?

I think the simplicity of the code is of more value than skiping 1 or 2 regular expressions during CMake configuration.

acolombier

LGTM! This seems to slow down the test run by about 30 seconds (for Ubuntu). That being said, the added value is worth the delay IMO.

Giving some time to let other member a chance to review this as well.

JoergAtGithub · 2026-02-23T17:14:28Z

Merge?

Fixed wrong and duplicated mapping name

b428c43

github-actions Bot added build code quality controllers labels May 25, 2024

Use parametric GTest for mapping validation, instead of one test for …

2dc1f20

…all mappings

JoergAtGithub force-pushed the parametricGTestForMappings branch from 7931539 to 2dc1f20 Compare May 25, 2024 13:49

Improved output by suppressing the hex dump of MappingInfo as parameter

2a0789b

Fixed clazy error

JoergAtGithub force-pushed the parametricGTestForMappings branch 3 times, most recently from 5e3e7cf to 58f0867 Compare May 26, 2024 17:16

Use gtest_discover_tests instead of gtest_add to resolve the parametr…

c85d371

…ized gtests

JoergAtGithub force-pushed the parametricGTestForMappings branch from fb7709b to c85d371 Compare May 26, 2024 19:22

JoergAtGithub added 2 commits May 26, 2024 23:02

Fixed QT_QPA_PLATFORM setting, which was not defined in build-check m…

ad6201d

…atrix

Merge remote-tracking branch 'upstream/main' into parametricGTestForM…

a2de90e

…appings

JoergAtGithub force-pushed the parametricGTestForMappings branch from bd81c8f to a2de90e Compare May 26, 2024 22:40

daschuer added controller backend and removed controllers labels Jun 25, 2024

github-actions Bot added the stale Stale issues that haven't been updated for a long time. label Sep 24, 2024

Swiftb0y reviewed Sep 24, 2024

View reviewed changes

Comment thread src/test/controller_mapping_validation_test.h Outdated

Comment thread src/test/controller_mapping_validation_test.cpp Outdated

Comment thread src/test/controller_mapping_validation_test.cpp Outdated

github-actions Bot removed the stale Stale issues that haven't been updated for a long time. label Sep 25, 2024

JoergAtGithub mentioned this pull request Oct 15, 2024

LegacyControllerMappingValidationTest.MidiMappingsValid ctest timeout under Address Sanitizer option. #13771

Closed

JoergAtGithub added 2 commits December 21, 2024 10:59

Merge remote-tracking branch 'upstream/main' into parametricGTestForM…

207a818

…appings

Removed code that tdepends on mixxxdb mixxxtest. Builds again but run…

71c5cbe

…s slow

github-actions Bot added the stale Stale issues that haven't been updated for a long time. label Jan 23, 2025

github-actions Bot removed the stale Stale issues that haven't been updated for a long time. label Sep 7, 2025

JoergAtGithub added 3 commits November 22, 2025 18:23

Fixed wrong and duplicated mapping name

914c31d

Use parametric GTest for mapping validation, instead of one test for …

2416567

…all mappings

Improved output by suppressing the hex dump of MappingInfo as parameter

cfe169f

Fixed clazy error

WIP: compute mapping list in CMake

13611c3

JoergAtGithub changed the base branch from main to 2.6 February 7, 2026 20:23

JoergAtGithub added 5 commits February 8, 2026 11:03

Merge remote-tracking branch 'upstream/2.6' into parametricGTestForMa…

fe03493

…ppings

Set RESOURCE_FOLDER path as compile definition for mixxx-test in CMake

5827d47

Use CMake to collect mapping files at configure time and pass them compile definition.

Re-add accidentally deleted setup code for QML

9049618

Fix clazy

02cdcba

github-actions Bot added the controller mappings label Feb 8, 2026

JoergAtGithub marked this pull request as ready for review February 8, 2026 23:28

acolombier reviewed Feb 8, 2026

View reviewed changes

JoergAtGithub added 2 commits February 9, 2026 20:58

Remove commented code section

4d55f55

Use Qt include aliases

df61ae9

acolombier approved these changes Feb 9, 2026

View reviewed changes

acolombier mentioned this pull request Feb 15, 2026

Reduce the CI footprint #14297

Open

2 tasks

acolombier merged commit e45fc6f into mixxxdj:2.6 Feb 23, 2026
14 checks passed

JoergAtGithub deleted the parametricGTestForMappings branch February 23, 2026 21:20

acolombier added this to Releases Feb 25, 2026

github-project-automation Bot moved this to In progress in Releases Feb 25, 2026

github-project-automation Bot moved this from In progress to Done in Releases Feb 25, 2026

acolombier added this to the 2.6.0 milestone Feb 25, 2026

	if (coScript == nullptr) {
	// The test setups do not run all of Mixxx, so ControlObjects not
	// existing during tests is okay.
	if (!m_pScriptEngineLegacy->isTesting()) {
	m_pScriptEngineLegacy->logOrThrowError(
	QStringLiteral("script tried to connect to ControlObject "
	"(%1, %2) which is non-existent.")
	.arg(group, name));
	}
	return QJSValue();
	}

	if (!callback.isCallable()) {
	m_pScriptEngineLegacy->logOrThrowError(QStringLiteral(
	"Tried to connect (%1, %2) to an invalid callback. Make sure "

Uh oh!

Conversation

JoergAtGithub commented May 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 24, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jan 23, 2025

Uh oh!

acolombier commented Nov 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JoergAtGithub commented Nov 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Swiftb0y commented Nov 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JoergAtGithub commented Nov 22, 2025

Uh oh!

Swiftb0y commented Nov 22, 2025

Uh oh!

acolombier commented Nov 23, 2025

Uh oh!

JoergAtGithub commented Nov 23, 2025

Uh oh!

acolombier commented Nov 23, 2025

Uh oh!

JoergAtGithub commented Feb 8, 2026

Uh oh!

acolombier left a comment

Choose a reason for hiding this comment

Uh oh!

acolombier Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

JoergAtGithub Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

acolombier Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

JoergAtGithub Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

acolombier left a comment

Choose a reason for hiding this comment

Uh oh!

JoergAtGithub commented Feb 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JoergAtGithub commented May 25, 2024 •

edited

Loading

acolombier commented Nov 22, 2025 •

edited

Loading

JoergAtGithub commented Nov 22, 2025 •

edited

Loading

Swiftb0y commented Nov 22, 2025 •

edited

Loading